Expose enableOnDemandInstructionDiscovery across all SDK SessionConfig types#1323
Open
examon wants to merge 2 commits into
Open
Expose enableOnDemandInstructionDiscovery across all SDK SessionConfig types#1323examon wants to merge 2 commits into
examon wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new session configuration flag (enableOnDemandInstructionDiscovery) across the in-repo SDKs so consumers can opt into (or explicitly disable) runtime on-demand custom-instruction discovery via the JSON-RPC wire key enableOnDemandInstructionDiscovery.
Changes:
- Exposes the new flag on SessionConfig + ResumeSessionConfig surfaces (and forwards it on
session.create/session.resume) for Node, Python, Go, .NET, and Rust. - Adds/extends unit + E2E coverage to verify
true/falseserialization and key omission when unset. - Adds inline API documentation for the new option in each language’s config surface.
Show a summary per file
| File | Description |
|---|---|
| rust/tests/session_test.rs | Adds serde wire-format tests for enableOnDemandInstructionDiscovery on SessionConfig/ResumeSessionConfig. |
| rust/src/types.rs | Adds enable_on_demand_instruction_discovery: Option<bool> + builder methods + debug/default updates and unit-test coverage. |
| python/test_client.py | Adds forwarding tests ensuring explicit False is sent on create/resume. |
| python/e2e/test_client_options_e2e.py | Extends E2E options propagation test to include the new flag. |
| python/copilot/client.py | Adds new kwargs to create/resume APIs, docs, and wire payload serialization. |
| nodejs/test/e2e/client_options.e2e.test.ts | Extends E2E options propagation test to assert the new flag is present. |
| nodejs/test/client.test.ts | Adds unit tests verifying forwarding on session.create and session.resume. |
| nodejs/src/types.ts | Adds enableOnDemandInstructionDiscovery?: boolean to SessionConfig and includes it in ResumeSessionConfig pick. |
| nodejs/src/client.ts | Forwards the new flag in create/resume request payloads. |
| go/types.go | Adds *bool config fields and wires them into create/resume request structs (plus gofmt alignment). |
| go/internal/e2e/client_options_e2e_test.go | Extends Go E2E options propagation assertions to include the new flag. |
| go/client.go | Forwards EnableOnDemandInstructionDiscovery into create/resume wire requests. |
| go/client_test.go | Adds wire-format unit tests for explicit true/false and omission behavior. |
| dotnet/test/Unit/SerializationTests.cs | Adds serialization tests validating true/false/omitted behavior for create/resume requests. |
| dotnet/test/Unit/CloneTests.cs | Ensures SessionConfig/ResumeSessionConfig clone operations copy/preserve the new nullable property. |
| dotnet/src/Types.cs | Adds bool? EnableOnDemandInstructionDiscovery to SessionConfig/ResumeSessionConfig and copies it in clone constructors. |
| dotnet/src/Client.cs | Threads the new option through Create/Resume request construction and record types. |
Copilot's findings
- Files reviewed: 17/17 changed files
- Comments generated: 2
This comment has been minimized.
This comment has been minimized.
Mirrors the existing enableConfigDiscovery and remoteSession precedents (PRs #1044 and #1295). Exposes the SDK option that lets the runtime discover custom instruction files on demand after the agent reads or views files, complementing the existing up-front load of `.github/copilot-instructions.md`, `AGENTS.md`, etc. Wire key (camelCase, identical across all 5 SDKs): enableOnDemandInstructionDiscovery Type shapes: Node enableOnDemandInstructionDiscovery?: boolean Python enable_on_demand_instruction_discovery: bool | None = None Go EnableOnDemandInstructionDiscovery *bool .NET bool? EnableOnDemandInstructionDiscovery Rust Option<bool> with #[serde(skip_serializing_if = "Option::is_none")] Wire semantics: when set, the wire payload carries the literal value (including explicit `false`); when omitted, the key is dropped. Applies to both session.create and session.resume so callers can toggle the setting on a resumed session. Runtime-gated. The runtime honors the option only when custom instructions are enabled and the connected runtime supports on-demand custom instruction discovery; otherwise the option is accepted but no-ops. The SDK does not attempt to detect the runtime gate. Requires @github/copilot ^1.0.49-1 (the runtime change shipped in github/copilot#7759). Security: discovered instruction files are treated as model instructions and may be stored or replayed with session history. Docstrings caution against enabling for untrusted content, CI jobs processing untrusted forks, or directories writable by untrusted users or processes. Go shape note: uses *bool (not bool) so consumers can disable a previously-enabled session on resume. Reuses the precedent already set by EnableSessionTelemetry *bool and IncludeSubAgentStreamingEvents *bool. Does not retrofit the existing EnableConfigDiscovery bool field (that would be a separate breaking source change). Tests: each SDK adds tests for the new field on both create and resume, asserting that explicit `false` is serialized as `false` and that omission drops the key from the payload. Mirrors the test patterns already in place for enable_session_telemetry, include_sub_agent_streaming_events, and enable_config_discovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses review feedback on PR #1323: the create-session API docs for enable_on_demand_instruction_discovery / EnableOnDemandInstructionDiscovery should not include resume-specific behavior. That note already lives on the resume-side configs (Python resume_session and Go ResumeSessionConfig).
examon
force-pushed
the
tmeszaros/sdk-on-demand-instruction-discovery
branch
from
07bfd0a to
e9e961a
Compare
Contributor
Cross-SDK Consistency Review ✅This PR consistently exposes Consistency matrix verified
Known gaps (pre-existing, acknowledged in PR description)
ConclusionNo new cross-SDK consistency issues introduced by this PR. The implementation correctly follows the precedents set by
|
stephentoub
reviewed
May 19, 2026
| public bool? EnableConfigDiscovery { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// When <see langword="true"/>, requests on-demand discovery of custom instruction |
Collaborator
There was a problem hiding this comment.
What does "on-demand discovery" actually mean? Demanded by what / when, discovered where, etc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exposes the new
enableOnDemandInstructionDiscoveryconfiguration flag across all five in-repo SDKs (Node, Python, Go, .NET, Rust). Mirrors the precedent set by #1044 (enableConfigDiscovery) and #1295 (remoteSession).The runtime change shipped in github/copilot#7759 and is available in
@github/copilot@^1.0.49-1.Cross-language consistency matrix
falsebehaviorenableOnDemandInstructionDiscovery?: booleanenableOnDemandInstructionDiscoveryfalseenable_on_demand_instruction_discovery: bool | None = NoneenableOnDemandInstructionDiscoveryfalseEnableOnDemandInstructionDiscovery *boolenableOnDemandInstructionDiscoverynil)falsebool? EnableOnDemandInstructionDiscoveryenableOnDemandInstructionDiscoveryfalseenable_on_demand_instruction_discovery: Option<bool>enableOnDemandInstructionDiscoveryfalseRuntime-gated availability
When set to
true, the SDK asks the runtime to discover custom instruction files on demand after the agent successfully reads or views files. The option is runtime-gated: it takes effect only when custom instructions are enabled and the connected runtime supports and enables on-demand custom instruction discovery. Otherwise the runtime accepts the option but performs no on-demand discovery (silent no-op, no error or warning).The SDK does not attempt to detect runtime gating; it forwards the option unconditionally. On
session.resume, omitting the option leaves the existing session setting unchanged, and passing an explicitfalsedisables future on-demand discovery for that resumed session.Trust and security
Enable only for trusted repositories or workspaces. Discovered instruction files are treated as model instructions, may affect agent behavior, and may be stored or replayed with session history. Do not enable for untrusted content, CI jobs processing untrusted forks, or directories writable by untrusted users or processes.
Go shape
Uses
*bool(notbool) on bothSessionConfigandResumeSessionConfigso callers can disable a previously-enabled session on resume. Reuses the precedent already set byEnableSessionTelemetry *boolandIncludeSubAgentStreamingEvents *bool. This PR does not retrofit the existingEnableConfigDiscovery boolfield — that would be a separate breaking source change with broader blast radius.Tests
Each SDK adds tests for the new field on both
session.createandsession.resume, asserting that explicitfalseis serialized on the wire and that omission drops the key entirely. Mirrors the test patterns already in place forenable_session_telemetry,include_sub_agent_streaming_events, andenable_config_discovery.nodejs/test/e2e/client_options.e2e.test.tshappy-path; adds 2 unit tests innodejs/test/client.test.ts(create + resume, explicitfalse).python/e2e/test_client_options_e2e.pyhappy-path; adds 2 unit tests inpython/test_client.py(create + resume, explicitFalse).go/internal/e2e/client_options_e2e_test.gohappy-path; adds 6 wire-format unit tests ingo/client_test.go(create + resume;Bool(true),Bool(false), and omitted).dotnet/test/Unit/SerializationTests.cs(create + resume; true, false, omitted) and 4 clone tests indotnet/test/Unit/CloneTests.cscovering the copy-ctor.rust/tests/session_test.rs(create + resume;Some(true),Some(false), omitted) and extends the existing builder unit-tests inrust/src/types.rsto call the new.with_enable_on_demand_instruction_discovery(...)method on bothSessionConfigandResumeSessionConfig.Documentation
The new field is documented inline in each SDK's typed config surface using the language-appropriate doc-comment style (JSDoc, Google-style, godoc, XML, Rustdoc). README and CHANGELOG are intentionally not updated, matching the precedent of #1044 and #1295.
Notes for reviewers
go/types.goshows ~170 line changes; the bulk is gofmt-driven struct-tag column re-alignment because the new field name is longer than any existing field. The only semantic changes are the additions ofEnableOnDemandInstructionDiscoverytoSessionConfig,ResumeSessionConfig, and the two wire-request structs.enableConfigDiscoverydocstring statement that custom instruction files "are always loaded from the working directory regardless of this setting" remains accurate — the new option adds on-demand discovery on top of the up-front load; it does not replace it.python/copilot/session.pyTypedDicts do not include the new field. They are also missing existing fields (enable_config_discovery,remote_session); closing those gaps is best handled in a follow-up so this PR stays narrow.